[ETL-631] Update index fields with ParticipantIdentifier and propagate participant id fields
#110
Conversation
And a small update to FitbitEcg table schema that should have been included when we originally added this data type.
|
| "garminhrvsummary": ["ParticipantID", "StartTimeInSeconds"], | ||
| "garminmanuallyupdatedactivitysummary": ["ParticipantID", "SummaryId"], | ||
| "garminmoveiqactivitysummary": ["ParticipantID", "SummaryId"], | ||
| "garminpulseoxsummary": ["ParticipantID", "SummaryId"], |
There was a problem hiding this comment.
Was ParticipantID the wrong key?
There was a problem hiding this comment.
There's a one-to-one mapping from ParticipantIdentifier to ParticipantID. They are two different ways of representing the same identifier.
ParticipantIdentifier is the only field present in every data type (including SymptomLog) and the CE docs suggest it's the more "official" identifier -- kind of like HealthCode to ExternalId in mPower.
| ).distinct() | ||
| index_fields = INDEX_FIELD_MAP[table_data_type] | ||
| additional_fields = [selectable_original_field_name, "cohort"] | ||
| if "ParticipantID" in parent_table.columns: |
There was a problem hiding this comment.
Should we modify /add a new test for this function to check this part that ParticipantID gets included in additional_fields if it exists? I was reading the JIRA ticket/slack thread - why would both ParticipantID and ParticipantIdentifier exist in a dataset?
There was a problem hiding this comment.
Ahh, yes we should. I'm always forgetting about tests 🤦
There was a problem hiding this comment.
why would both ParticipantID and ParticipantIdentifier exist in a dataset?
I don't know the exact reason, but it's not unusual to have one be the "global" identifier and the other to be a study or app specific identifier.
There was a problem hiding this comment.
That's not confusing at all :D
| + INDEX_FIELD_MAP[table_data_type] | ||
| ) | ||
| ).distinct() | ||
| index_fields = INDEX_FIELD_MAP[table_data_type] |
There was a problem hiding this comment.
Given that now Participant_Identifier is a required index field for every data type (as far as I can tell in INDEX_FIELD_MAP), I'm thinking maybe we could have a test to ensure that Participant_Identifier is in all the key-value pairs in the dict of INDEX_FIELD_MAP. This is more of future proofing/double checking that we don't modify code that would accidentally affect this. Thoughts?
There was a problem hiding this comment.
I don't want to assume that ParticipantIdentifier will be included with every data type going forward. That's a CE decision, and why I explicitly included the field in the INDEX_FIELD_MAP rather than specifying it once in this function.
ParticipantIDfield, propagate that to the child